West Midlands | ITP-Sept-2025 | Ali Naru | Sprint 3 | Coursework/sprint 3 implement and rewrite #845
Conversation
MohammedNaru
force-pushed
the
coursework/sprint-3-implement-and-rewrite
branch
4 times, most recently
from
e7b4c8b to
d40619a
Compare
MohammedNaru
force-pushed
the
coursework/sprint-3-implement-and-rewrite
branch
from
d40619a to
5dc9452
Compare
MohammedNaru
force-pushed
the
coursework/sprint-3-implement-and-rewrite
branch
from
9ac8d6e to
6082322
Compare
…corresponding tests
…lete assertions for all test cases
… face cards, and invalid inputs
…ds, face cards, Aces, and invalid inputs
MohammedNaru
added
Needs Review
cjyuan
added
Review in progress
| if (Math.abs(numerator) < denominator) { | ||
| return true; | ||
| } else { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
The spec wasn't clear enough.
How would you change your code if denominator can also be negative?
There was a problem hiding this comment.
Because the denominator might also be negative, we should compare absolute values. A proper fraction is defined by the absolute value of the numerator being smaller than the absolute value of the denominator, regardless of sign.
| if (!isNaN(rank)) { | ||
| return Number(rank); | ||
| } |
There was a problem hiding this comment.
In JavaScript, strings that represent valid numeric literals in the language can be safely
converted to equivalent numbers or parsed into a valid integers.
For examples, "0002", "0x04", "3.1416", "1111", "3e0".
Do you want to recognize these string values as valid ranks?
Does your function behave the way you expect when rank is one of these values?
There was a problem hiding this comment.
I updated the getCardValue function so it now correctly handles numeric cards, face cards, and Aces, and added proper validation with an error for invalid ranks. The previous version only covered a single case, but this update ensures full coverage for the rules
| // When the numerator is negative but absolute value < denominator, | ||
| // Then the function should return true | ||
| test("should return true for a negative proper fraction (|numerator| < denominator)", () => { | ||
| expect(isProperFraction(-4, 7)).toEqual(true); | ||
| }); |
There was a problem hiding this comment.
To make the tests cover all possible categories, you can consider including a test for negative improper fractions.
There was a problem hiding this comment.
I’ve updated the implementation based on feedback to make it more robust and complete. For isProperFraction, I added an additional test case to cover negative improper fractions, ensuring full coverage across all fraction categories. For getCardValue, I improved the rank parsing by validating numeric ranges properly rather than relying on a fixed list, and clarified handling of both face cards and invalid input by adding stricter validation. This should now behave more reliably and align better with real-world card value rules. Let me know if any further tweaks are needed!
cjyuan
added
Reviewed
…e test organization
…nction with rank handling
MohammedNaru
added
Needs Review
…es and improve error handling
…inal cases and improve error handling" This reverts commit b489194.
2 participants
Coursework/sprint 3 implement and rewrite
Self checklist
Changelist
Pull request complete, all tests working and passed successfully
Questions
no current questions for review.